Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

The Miri subtree was changed

cc @rust-lang/miri

@rustbot labels +T-lang

If the idea with this is that passing -Cmin-function-alignment represents a language-level guarantee, e.g. that a naked function could validly rely on this minimum alignment holding when this flag is passed to prove the soundness of its operations -- that from a language spec standpoint, this is equivalent to marking each function with #[align(..)] -- then I'd suggest that a dual FCP with lang here is proper.

cc @rust-lang/lang

Yeah that sounds right. There is also a bunch of overlap with rfc 3806.

Specifically, it contains a section that attempts to define what alignment values are accepted. #[align] and -Cmin-function-alignment should accept/reject the same values.

@rust-lang/spec: What thoughts do we have about how to handle compiler flags that affect the language definition?

cc @RalfJung

Here's a question. Since this does have language-level effect, might this be better expressed as a crate-level attribute?

If there's a good reason, given the use case, for this to be a compiler flag instead, probably it'd be good to describe that in some detail.

If it can then be applied via --crate-attr similarly, then that should be fine for our use case.

Huh. That could be... interesting to work with, since it would logically apply only to entities that originate from that crate, right?

@folkertdev Do any of our tests verify that if you

• use this option on the functions described in a crate
• one is generic or from a trait or otherwise non-instantiated
• you instantiate that function in another crate without this flag

Then the resulting function is instantiated with the correct alignment? Where "correct" is... well, pick an answer, I guess?

☔ The latest upstream changes (presumably #142901) made this pull request unmergeable. Please resolve the merge conflicts.

@workingjubilee we don't, currently. I think the expected answer is clear though, because -Zmin-function-alignment is applied to all compiled crates, not just the top-level one?

I'm not sure how that would work with #![min_function_alignment = N], because then library crates could set the value as well? So far I've been thinking of this option as similar to -Ctarget-cpu or -Ctarget-feature: they are not properties of a crate but of the compilation as a whole.

What thoughts do we have about how to handle compiler flags that affect the language definition?

Is setting and then relying on the alignment fundamentally different from -Ctarget-feature=+avx2 and then asserting in the program that avx2 is available?

Is setting and then relying on the alignment fundamentally different from -Ctarget-feature=+avx2 and then asserting in the program that avx2 is available?

It's about whether we like this as a safety argument:

/// SAFETY: The pointee must be a 32-byte aligned function.  It's OK
/// for the low-order bits of the function pointer itself to be
/// non-zero, e.g., if they're used for controlling the processor
/// mode.
unsafe fn f(x: fn()) { todo!() }
fn g() {
    let p = g as fn();
    // SAFETY: We compiled with `-Cmin-function-alignment=32`.
    unsafe { f(p) };
}

For avx2, I'd expect that the safety argument would be made by use of cfg(target_feature = ".."), if is_x86_feature_detected!("avx2") { .. }, etc., rather than by using the argument that it was compiled with -Ctarget-feature=+avx2 in the proof.

they are not properties of a crate but of the compilation as a whole.

Wait so, does this need to be handled like Target Modifiers?

Some changes occurred in compiler/rustc_codegen_ssa/src/codegen_attrs.rs

cc @jdonszelmann

Wait so, does this need to be handled like Target Modifiers?

well, two crates with a different minimum alignment can work together just fine. In fact, this is usually the case because the standard library is pre-compiled and hence may not have the same minimum alignment value. It is only when the minimum alignment is relied on in unsafe code that you'd ever run into trouble.

The goal of -Cmin-function-alignment is performance: by aligning the code, you'll likely have fewer cache misses. When the specific alignment value matters for correctness, (e.g. for pointer tagging), I've only seen #[align(4)] or similar being used so far.

For avx2, I'd expect that the safety argument would be made by use of cfg(target_feature = ".."), if is_x86_feature_detected!("avx2") { .. }, etc., rather than by using the argument that it was compiled with -Ctarget-feature=+avx2 in the proof.

Similarly for alignment I'd expect something like (func as usize).is_multiple_of(alignment) (pending a better way of getting the function address).

Btw I don't particularly like the kind of safety argument in the example, but I think it is no worse than just assuming that a target feature is enabled.

I think it's entirely valid to use a compiler flag for performance concerns like this, but then the flag should probably be considered more as a hint than a guarantee. I actually like clang's naming better for this purpose.

I think for language-level guarantees we should probably use #[align] or possibly a crate-level attribute; that's a good callout by @traviscross.

I think there are gray areas; I could see there being situations where you need to apply something to a whole crate graph and are forced to rely on either compiler flags or linker tricks. As far as Rust is concerned I like --crate-attr as a way to make it clear when you are transitioning into the realm of these guarantees. I expect there are cases where we haven't followed this distinction and we'll need to clean it up over time.

The align value must be a power of 2, other values are rejected.

Do we actually have test coverage for this? I.e. -Cmin-function-alignment=0 or
-Cmin-function-alignment=3?

A min-function-alignment value lower than the target's minimum has no effect.

Should this be a warning? If this is intended as a hint, then 🤷.

The goal of -Cmin-function-alignment is performance: by aligning the code, you'll likely have fewer cache misses. When the specific alignment value matters for correctness, (e.g. for pointer tagging), I've only seen #[align(4)] or similar being used so far.

Doesn't LLVM already insert padding between functions for alignment by default? I believe it aligns to 16 bytes on x86_64 for example. Cranelift also does that.

Do we actually have test coverage for this? I.e. -Cmin-function-alignment=0 or -Cmin-function-alignment=3?

This PR adds tests for:

//@ revisions: too-high not-power-of-2
//
//@ [too-high] compile-flags: -Cmin-function-alignment=16384
//@ [not-power-of-2] compile-flags: -Cmin-function-alignment=3

//~? ERROR a number that is a power of 2 between 1 and 8192 was expected

The implementation uses Align::from_bytes to parse the value.

Should this be a warning? If this is intended as a hint, then 🤷.

The flag is deliberately specified as a minimum. If for some external reason the alignment is already higher, the flag still works as intended: the alignment is at least the specified value.

Doesn't LLVM already insert padding between functions for alignment by default? I believe it aligns to 16 bytes on x86_64 for example. Cranelift also does that.

Yes, there is a per-platform minimum, but RfL wants to bump it higher than than minimum in some cases.

From #t-compiler/help > ✔ Alignment for function addresses:

Asked because in Linux kernel, to enable dynamic function trace, on ARM64 the function addresses are required to be 8-byte aligned: https://github.com/Rust-for-Linux/linux/blob/rust-next/arch/arm64/kernel/ftrace.c#L95

Note the dynamic check for the address actually being aligned in the linked C code.

Yes, there is a per-platform minimum, but RfL wants to bump it higher than than minimum in some cases.

That case is not merely a performance optimization, but actually mandatory. For something mandatory a target modifier makes sense to me.

It is still not unsound, though, and based on the RFC summary:

A target modifier is a flag where it may be unsound if you link together two compilation units that disagree on the flag.

But maybe the definition of when a flag is a target modifier is broader in practice? I agree that in that case you'd want to effectively guarantee all codegen units use some minimum alignment.

If this is supposed to be a guarantee it's not obvious to me that a -C flag is the right way to do it.

If the goal is that this is for RFL, should RFL just have a different target with a different function alignment setting? Feels like they'd want a different target for things like their different float and vector-register-usage stuff too anyway.

With a -C flag it just brings up a whole bunch of questions to me about what's supposed to happen with, say, an inlined function that's defined in a crate compiled with -Cmin-function-alignment but actually monomorphized in a crate without that flag? Does that still give a "guarantee" about the alignment of it? If someone starts using -Z hint-mostly-unused -- especially if we autodetect it for dependencies, eventually -- does that make -Cmin-function-alignment mostly meaningless in that crate because we delay codegen to the use?

Obviously with a target flag that inconsistency can't happen, so it just feels nicer to me, and avoids a whole bunch of "well should we have a warning that you compiled a crate with a lower value that a dep?" kinds of questions.

If the goal is that this is for RFL, should RFL just have a different target with a different function alignment setting? Feels like they'd want a different target for things like their different float and vector-register-usage stuff too anyway.

If you mean adding a new built-in target, then we have avoided adding targets for things like this because there would be way too many due to combinatorial explosion and because it would tie us to new Rust releases if new values are needed in the future.

(There was a Linux kernel target years ago, and we didn't use it for those reasons. We ended up talking about "global target features" for things where we just wanted to modify the base target because we knew we would pass the same flags everywhere for a given build, and then finally we had "target modifiers" for things where the ABI really needs to match so it is checked.)

I don't know that this needs lang approval, personally. But I would say it would be useful to specify whether this option is a best-effort or a guarantee. And, if it's a guarantee, then how does it interact with inlining? In general, I would expect that if you take the address of an inline function then we'd make sure there's an entry point you can take the address of, which any guarantee could then apply to.

@workingjubilee

Then the resulting function is instantiated with the correct alignment? Where "correct" is... well, pick an answer, I guess?

I think #142854 actually changed behavior here. After that PR, the alignment of the crate that contains the function is used, and this gets embedded in the crate metadata as part of the function attributes. Before that PR, the alignment of the crate that codegen'd the function was used.

We talked about this today in lang triage without specific resolution. We discussed how we may need to see more details here about the use case and the specific ask, and may want to hear from other teams, particularly @rust-lang/opsem and @rust-lang/spec. Some of us have also left individual comments above.

From an opsem perspective, the only thing that's tricky here is what exactly we guarantee when mixing different values of this flag in crates that are linked together and form one AM. IMO the "obvious" answer is to use the flag set for the crate that contains the function is used (so, the flag indeed acts basically like a crate-level attribute), and I think that is the current implementation, but we have no tests for this case. Also, @tmandry expressed that they would expect a different semantics, though I don't know how to even phrase that one in AM terms.